Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial implementation of the top bar #951

Merged
merged 26 commits into from
Mar 27, 2022

Conversation

lmoureaux
Copy link
Contributor

This PR turns the side bar into a top bar. I did some refactoring and simplification of the side/top bar code along the way, which explains the source lines count.

This is intended as a first step so we can start creating issues about specific features. Bugs are expected and I'll only solve very easy ones as part of this PR. Theme support is left for later (currently, the widgets use the system theme).

Only tested with Nightstalker and a dark system theme (Breeze Dark to be specific).

Closes #940.
(Or if we consider 940 as a meta issue, closes the first child issue.)

@lmoureaux lmoureaux added enhancement New feature or request refactoring This issue requires code refactoring gui This issue requires changes to the user interface labels Mar 20, 2022
@lmoureaux lmoureaux added this to the v3.0-beta.2 milestone Mar 20, 2022
@psampathkumar
Copy link
Contributor

psampathkumar commented Mar 25, 2022

Looks good to me, just a couple of conflcts due to the recent NULL -> nullptr in master. I say, we merge it and then tweak the topbar as we go along. This is a good start.

@jwrober jwrober removed this from the v3.0-beta.2 milestone Mar 25, 2022
We decided that the contents of the sidebar would be shown above the map. Start
this change by moving the widget to the top.

See longturn#940.
QToolButton seems a better fit because it already provides most of the options
we need. Maybe the top bar should become a fully-fledged QToolBar.

See longturn#940.
Replace custom-drawn icons with a standard QToolButton feature. Now they're
even SVG-capable!

See longturn#940.
This simplifies the code a little.

See longturn#940.
We'll rethink the layout later; for now, simplify it.

See longturn#940.
It was used for a tiny graphical tweak of little importance.

See longturn#940.
Themes can use QSS if they want something special.

See longturn#940.
It's not perfect but we're not after perfection. It works.

See longturn#940.
It wasn't used, removing it simplifies the code.

See longturn#940.
I'm not sure what this will become so I'm implementing an ugly placeholder that
does the job.  We may want to implement the switching with a set of mutually
exclusive QAction.

See longturn#940.
Some of them can only be clicked (e.g. End Turn).

See longturn#940.
Use a specialized class instead of a special mode of top_bar_widget.

See longturn#940.
Use a near-copy of tax_rates_widget instead of a special mode of
top_bar_widget.

See longturn#940.
Code cleanup after moving the functionality to separate classes.

See longturn#940.
@lmoureaux
Copy link
Contributor Author

Rebased.

@lmoureaux lmoureaux enabled auto-merge (rebase) March 25, 2022 21:21
@jwrober
Copy link
Collaborator

jwrober commented Mar 25, 2022

I think the only thing this needs now is the client themes to support dark mode as I mentioned in Discord.

@lmoureaux lmoureaux disabled auto-merge March 25, 2022 21:25
The code was previously saying that it didn't want it to be drawn, leading to
artifacts with semitransparent buttons. Now the area is always cleared before
painting.

See longturn#951.
@lmoureaux
Copy link
Contributor Author

Theming done.

@jwrober jwrober merged commit 8f09a07 into longturn:master Mar 27, 2022
jwrober pushed a commit that referenced this pull request Mar 27, 2022
jwrober pushed a commit that referenced this pull request Mar 27, 2022
The code was previously saying that it didn't want it to be drawn, leading to
artifacts with semitransparent buttons. Now the area is always cleared before
painting.

See #951.
jwrober pushed a commit that referenced this pull request Mar 27, 2022
jwrober pushed a commit that referenced this pull request Mar 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request gui This issue requires changes to the user interface refactoring This issue requires code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move the left side bar to a horizontal top bar
3 participants